Codecov Report

Merging #229 into next will decrease coverage by 1.97%.
The diff coverage is 6.26%.

@@            Coverage Diff             @@
##             next     #229      +/-   ##
==========================================
- Coverage   32.67%   30.69%   -1.98%     
==========================================
  Files          46       46              
  Lines        8524     9128     +604     
==========================================
+ Hits         2785     2802      +17     
- Misses       5739     6326     +587     

First of all, big thanks for spending time on this.

It seems to me, at current stage, this pull request is fairly incomplete. Because it only adds rounded corners to the old xrender backend (we are moving away from the old backends). Also I am not sure if what's implemented here meets the criteria of #214 (that's for the issue reporter to decide, of course).

As you said, reg_ignore optimization is not used after your changes, I don't think that's good. It also removes some of the customization options for shadows (shadow-exclude-reg is gone, and full-shadow becomes enabled for all windows).

Also, some nitpicks:

• Can you squash the fixes into the original commit that implements rounded corners?
• Can you make sure you run clang-format on your code?
• Please document (what do they do. and if it's not obvious, also how they do it) the new functions you added (I know a lot of the functions aren't documented, but I want to make sure all new functions are).

Many thanks.

Thank you for working on this, @sdhand! That's an exciting update.

I've tested your branch a bit and it works nice in most cases. But as @yshui already said it should support at least the "glx" backend apart from the old "xrender" backend. I'm not familiar enough with compton architecture so I hope that @yshui will have time to review your updates regarding this particular point and other technical details.

What's about the criteria of #214:
Of course, that issue is mostly about proof of concept of technical implementation of corner rounding which is the hardest part, but aside from the main goal there is a next requirement:

• The window exclude/include rules like it's already done for shadows and blur

I understand that it should be the trivial thing for someone from community or for myself to implement but it would be nice to have that feature included in this PR, because it should be possible to disable rounding for panels, docks and some other special windows. At least, it should make all the bounty bakers satisfied with the result.

Other integration problems I've noticed so far:

• When the window background blur option is enabled the transparent area of a corner gets blurry background as well, but it should not.

@zezic window exclude/include rules should be pretty trivial, I'll add that in soon. I'm currently working on adding back the reg_include optimization. (Completely obscured areas of windows are currently drawn, to avoid corruption in the transparent corners. Fixing this requires me to properly calculate the region occupied by a window with a rounded corner).

Getting this to work with the other backends will be challenging, but I do plan on doing so eventually.

The window blur problem is likely slightly fiddly, but I should be able to solve it.

Here's a list of what remains to do:

• Get the reg_ignore optimization working

• Fix blur being applied to the transparent area of the corner

• Fix the assumption that windows are always rendered in one go (This is in particular not true when transparent frames are used).

• Implement include/exclude rules

• Document new functions

• Port to new backends

@sdhand yeah, I agree with you that exclude/include rules should be trivial. The implementation for other backends is not easy and the blur problem is also can be challenging though.
Anyway I'm happy to see the progress on this issue so I've posted another $10 for the bounty. It's not that much, but it's better than nothing :)
I think, it will be possible to post with an update to Reddit and take the another portion of attention after this PR gets more polished.

I've got a fix for both reg_ignore and the blur problem. I approximate the window bound by just cutting off a radius*radius square at each corner. This does mean that a few pixels in the corner are now no longer blurred, when in fact they should be (this is only noticeable if you choose to look for it, imo).

I could try and calculate a completely accurate window bound, but I'm not sure if this is worth the extra computational cost?

@sdhand I just tested the latest changes. As you said, there are little non-blurred quads at window corners now.

I think, that users who like to use big blur radius will be very unhappy to see such behavior. There is many of them. The difference between blurred area and non-blurred quad will be especially dramatic and noticeable after merging the kawase blur method from the tryone fork.

That's my opinion from the end-user point of view. I don't know, maybe this operation is not so performance-heavy at all or maybe it's possible to cache something as long as corner radius is the same for all windows? Maybe @yshui will be able to suggest something because I have no enough knowledge of all those xcb_ methods, unfortunately.

maybe it's possible to cache something as long as corner radius is the same for all windows?

if you use a mask picture to achieve the rounded corners, you can cache the mask the same way we cache the shadow picture.

Should this round shadows as well?

@GaugeK ideally - yes. But for now it would be better to leave them as is to make the rounded corners feature available as soon as possible.

After a while this seems to build up a fair bit of ram usage.

Apologies for the slow progress on this, I've been away and without access to a computer.

@yshui I am using a mask picture (current an xcb_render_picture_t), but even if I cached this, I'm not sure how I would obtain a region_t from it to correctly set the window bound, even if I were to find a solution, this would probably be making the code more xrender dependent, rather than less.

I imagine I just need to compute the region for the bound separately, but unfortunately it doesn't seem like pixman provides a way for building a region from triangles or trapezoids.

@GaugeK I've probably got a memory leak somewhere, will investigate.

@sdhand hmmm, yes, the ignored region thing is a bit tricky. but i don't think you need to be completely precise since it's just an optimization anyway. maybe you can use the biggest rectangle that can fit inside the rounded window (or just any rectangle inside the window) as the bound for ignored region (and for damages, use the non-rounded rectangle)?

@yshui currently I just use a rectangle the size of the window, with squares the length of the corner radius subtracted from each corner. Unless I am mistaken, the issue with this is the fact that this region is used to determine where to apply blur affects, meaning that a small area near each corner of a transparent window will not be blurred. (I personally don't think this is too noticeable unless you're looking for it, but @zezic did not seem to agree).

@sdhand ah that's simple, you just blur the whole window, then use your mask to clip the blurred image before putting it on screen

Oh I see, fantastic, I'll get that sorted.

For porting to the new backends, I think it's best to add a IMAGE_OP_MASK (or IMAGE_OP_CLIP?) to the image_op API that applied the alpha from a mask image to the image. Then you can use that API to clip the window and the blur.

I can confirm a memory leak :)

I can reproduce it by holding my resize keybinding and resizing some window for a while. My WM is Fluxbox. No, I was wrong. I'm not sure what action is causing it.

When using --experimental-backends on this fork it seems to make my fans speed up, works fine on yshui's compton.

I hope this gets implemented in the future, looks really cool!

@Baitinq I would like to see this too.

Hi! Any chance to see this soon? It looks really neat and seems fairly usable by now.

I added a rounded-corners-exclude configuration option to @sdhand's branch.

Example usage: Add the following lines to your configuration file in order to prevent Polybar and all windows of type dock from being rounded.

rounded-corners-exclude = [
    "class_g = 'polybar'",
    "window_type = 'dock'"
];

Pull request : sdhand#3

@sdhand Hi, just want to catch up. What is the current status of this PR? What is your plan with it?

#229 (comment)

@sdhand you might want to edit this to make the exclude bit checked.

@ibhagwan Can you rebase your branch? I will take a look of the code when I have time. The change is sizable so it might take a while.

@ibhagwan Also the commit history is a bit messy, it would be nice if you could re-organize them (e.g. squash relevant WIP commits together, merge fix commits into the commits they fix, etc.)

That will help me a lot in reviewing the code. Thanks.

@ysui I can certainly do so (rebase) on my own fork but my one contains the dual kawase blur methods from @tryone144 forks which we wanted to separate.

Only the last 3 commits in @sdhand‘a fork are mine, even though @sdhand gave me permissions on his repo I’d prefer not to mess with his repo too much with rebasing and other git manipulations as I’m not sure what other work is dependent on his repo.

Perhaps @sdhand is better suited to rebase his own repo?

@ibhagwan make senses, let's wait for @sdhand to reply then.

@sdhand Hi, this PR seems to be in good shape, and I am willing to merge it. The only thing is that I prefer to maintain a clean git commit history in this repo. Can you re-organize your branch, or allow @ibhagwan to do so?

Many thanks!

Yeay I'm exited to finally get rounded corners merged!

Yeay I'm exited to finally get rounded corners merged!

x2

Yeay I'm exited to finally get rounded corners merged!

x3. But will there be a patch allowing rounded shadows?

Alright, since I didn't get a reply, I will now be taking on the task of cleaning this PR up and getting this merged.

Hey sorry if this isn't meant to be functional yet but for some reason rounded borders aren't working? My window manager is herbstluftwm, the conf I'm using is

rounded-corners-exclude = [
    "class_g = 'lemonbar'",
    "window_type = 'dock'"
];
vsync = true;
backend ="glx";
shadow-offset-x = 5;
shadow = false;
shadow-radius = 0;
shadow-ignore-shaped = false;
shadow-offset-y = 5;
corner-radius = 15;
round-borders = 15;
frame-opacity = 50;

setting round-borders to 1 appears to make no difference. (Although it does clearly trigger a picom reload)
this is what the results of that conf look like:

Also it doesn't appear to work with hybrid backend either.

Hey sorry if this isn't meant to be functional yet but for some reason rounded borders aren't working? My window manager is herbstluftwm, the conf I'm using is

rounded-corners-exclude = [
    "class_g = 'lemonbar'",
    "window_type = 'dock'"
];
vsync = true;
backend ="glx";
shadow-offset-x = 5;
shadow = false;
shadow-radius = 0;
shadow-ignore-shaped = false;
shadow-offset-y = 5;
corner-radius = 15;
round-borders = 15;
frame-opacity = 50;

setting round-borders to 1 appears to make no difference. (Although it does clearly trigger a picom reload)
this is what the results of that conf look like:

Also it doesn't appear to work with hybrid backend either.

I’m not sure if it’s supposed to work as I’m not sure how Herbsluftwm draws its borders, from the image it seems non standard (due to the inner border 90 degree angles) - in this case you can overwrite the WM border and have picom draw its own rounded border by using the round-borders-rule config option, you can see an example in picom.conf.sample

Apologies but I don't really understand this syntax, I'm testing this with my terminal by doing round-borders-rule = [ "42:class_g = 'Alacritty'" ]; but it hasn't done anything unfortunately. Can you clarify? I see that it's PIXELS:PATTERN but I'm unsure of what PATTERN is meant to be, thanks. PIXELS is the desired border width though right?

Apologies but I don't really understand this syntax, I'm testing this with my terminal by doing round-borders-rule = [ "42:class_g = 'Alacritty'" ]; but it hasn't done anything unfortunately. Can you clarify? I see that it's PIXELS:PATTERN but I'm unsure of what PATTERN is meant to be, thanks. PIXELS is the desired border width though right?

You’re doing it correctly, 42 is the pixel width of the border, just a wild guess, perhaps herbsluftwm draws its border after picom thus overwriting the picom border? What happens if you set a bigger pixel width (just for testing), say 100 pixels? Can you test your Alacritty rule with the an exclude is your class_g accurate based on xprop output?

i've tested -- for me it works only for values btween 1 and 4-5

starting from 4-5 to 7 it works, but inner border radius is slighty smaller than expected

and starting from 8 inner border radius dissapers

i wasn't looking into the code, but seems to be it has smth to do with the formula for inner border shape

UPD: ive used corner-radius = 10; for testing and round-borders-rule = ["6:window_type = 'normal'"]; (or instead of 6 any values from 1 to 10, as described above)

i've tested -- for me it works only for values btween 1 and 4-5

starting from 4-5 to 7 it works, but inner border radius is slighty smaller than expected

and starting from 8 inner border radius dissapers

i wasn't looking into the code, but seems to be it has smth to do with the formula for inner border shape

UPD: ive used corner-radius = 10; for testing and round-borders-rule = ["6:window_type = 'normal'"]; (or instead of 6 any values from 1 to 10, as described above)

I'll have to look into it, here's how mine appears with border width set to 55, it does work but it also has the 90 degree internal angles, perhaps it's indeed the shader formula, I will have to look at the code.

I believe all the features implemented for the legacy backends here exist in the next branch now: rounded corners in xrender/glx backends, and rounded border in glx backend.

So I will close this PR now, porting of this to the new backends is still in progress.

@yshui corner-radius option indeed works with legacy backend but i didnt figured out a way to do it for the experimental backend

@actionless Yes, implementing this for the experimental backends is still in progress.

ah, sorry, i understood it like it's there but unstable

Hello,

I've been wanting to try the rounded corner features for my i3-gaps setup. So i compiled picom's next branch and installed it. But i've been unable to get proper the rounded corners to play nice with i3-gaps borders.

https://imgur.com/hQg16jr.

As you can see from this screenshot, the i3 borders are truncated and I'm not quite sure how to fix it. The screenshot also contains my current rounded corner settings. Does anyone have any idea on why it doesn't work for me?

thanks

Hello,

I've been wanting to try the rounded corner features for my i3-gaps setup. So i compiled picom's next branch and installed it. But i've been unable to get proper the rounded corners to play nice with i3-gaps borders.

https://imgur.com/hQg16jr.

As you can see from this screenshot, the i3 borders are truncated and I'm not quite sure how to fix it. The screenshot also contains my current rounded corner settings. Does anyone have any idea on why it doesn't work for me?

thanks

AFAIK, the main fork hasn’t implemented round-borders-rule yet.i3 uses a proprietary method to draw its borders which isn’t detected by picom so it doesn’t know it needs to round the borders. If you want to fix this you’ll need to use my fork and manually specify every window you’d like to have it’s borders rounded with round-borders-rule (look at the sample config in my fork for instructions).